Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v634][RF] Backport recent RooFit fixes to the release branch #16960

Merged
merged 4 commits into from
Nov 15, 2024

Conversation

guitargeek
Copy link
Contributor

Backport recent RooFit fixes to the release branch.

cburgard and others added 4 commits November 15, 2024 18:38
RooCmdArg is a bit of an old-style piece of code that doesn't really
work well with python. Also, when talking among statistics code
developers, it's commonplace to have to "exchange" fit arguments between
codes ("what arguments to you pass to make it converge?").

For this purpose, it's very convenient to:

  * be able to print the command arguments in a human-readable format,
    and

  * directly use these printouts to copy&paste them into some other
    piece of code to make comparison studies

The changes in this commit make this possible with little effort.
Before, we assumed that the "nominal" input always has the output size,
but that's not generally true. Any of the inputs, including "nominal",
might be a scalar that has to be broadcasted.
In older ROOT versions, RooMinimizer::FitResult used to have a method
called GetCovarianceMatrix. This commit reinstates that old method for
compatibility reasons.
As noted in GitHub issue root-project#7103, the interpolation code 3 is the same as
code 2 in the `FlexibleInterpVar` and the `PiecewiseInterpolation`
classes.

According to some comments in the source code, interpolation code 3 was
meant to be "a parabolic version of log-normal".

There were two options to fix this:

1) Actually implement this parabolic interpolation with linear
   extrapolation, analogous to code 2 but in log space.

2) Clearly mark interpolation code 3 as non-existing.

This commit implements solution 2, because the interpolation code 3 is
not mentioned anywhere outside the ROOT source code. Especially not is
the HistFactory paper:
https://cds.cern.ch/record/1456844/files/CERN-OPEN-2012-016.pdf

Implementing a new interpolation scheme that apparently was
not needed in the last 10 years anyway would increase the burden on the
user to understand all these different settings unnecessarily.

Closes root-project#7103.
@guitargeek guitargeek self-assigned this Nov 15, 2024
@guitargeek guitargeek merged commit 09da525 into root-project:v6-34-00-patches Nov 15, 2024
20 of 22 checks passed
@guitargeek guitargeek deleted the roofit-backports branch November 15, 2024 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants